Ema/prevent focus extensions #270
Merged
+242
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
GioSensation
commented
Mar 8, 2023
GioSensation
commented
Mar 8, 2023
GioSensation
commented
Mar 8, 2023
GioSensation
commented
Mar 8, 2023
GioSensation
commented
Mar 8, 2023
GioSensation
commented
Mar 8, 2023
|
@shakyShane I haven't added tests yet, but I'd appreciate a first pass. Please advise if you can think of how to test the Dax click thingy. I was thinking of just specifying click coords in the Playwright tests, but if you have a simpler idea I'm all ears 👂🙂👂. |
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
…b.com:duckduckgo/duckduckgo-autofill into ema/prevent-focus-extensions Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> # Conflicts: # dist/autofill-debug.js # dist/autofill.js # swift-package/Resources/assets/autofill-debug.js # swift-package/Resources/assets/autofill.js
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
GioSensation
commented
Mar 14, 2023
| @@ -45,6 +45,7 @@ function createDevice () { | |||
| } | |||
| return new AppleDeviceInterface(globalConfig, deviceApi, settings) | |||
| } | |||
| globalConfig.isExtension = true | |||
There was a problem hiding this comment.
I ended up setting it here. All other similar properties are in the globalConfig so it makes sense, but extensions don't inject strings on the fly so we can do it like this here. Any egregious downside?
FYI, using instanceof ExtensionInterface couldn't be used because imports were not failing.
There was a problem hiding this comment.
I'd say for now this is fine given how we're determining that we're in the extension - as you can imagine there are other ways to be more deterministic, but given what we have currently I don't think this is an issue at all :)
shakyShane
approved these changes
Mar 14, 2023
alistairjcbrown
added a commit
that referenced
this pull request
Mar 23, 2023
* Prevent in-context signup showing on local or invalid domain pages (#263) * Don't open in-context signup on input click (#264) * Show grayscale Dax icon for in-context signup (#265) * Remove initial dismissal in favor of single permanent dismissal (#266) * Only show in-context signup if recently installed (#267) * Adds a close X button to the top right of the tooltip (#268) * Fix grayscale dax logo (#271) * Ema/prevent focus extensions (#270) * Update icon asset to fix grayscale version (#276) * Improve narrow width iframes (#273) * Add Email Protection tooltip pixel (#277) * Fade Dax icon in on hover for in-context signup (#278) * Fade Dax icon in on hover for in-context signup * Keep Dax icon with active styles when tooltip open * Prevent icon flicker when clicking Dax icon for open tooltip * Move style management into form * Position tooltip above input when no space below (#282) * Position tooltip above input when no space below * Preload webfonts before showing tooltip This prevents the fonts loading in once the tooltip is shown and causing a layout shift. This is more important when the tooltip is showing above the input, as it can cause the tooltip to jump up. * Check tooltip position after revealed and bail if called before When running on slower devices, the tooltip position may be checked before the tooltip is revealed. When this happens, we now bail early without updating the position. Check position is now called after styles have loaded, when we know the tooltip is visible. * Hide caret initially and reduce position calculation calls * Read hidden attribute from DOM Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Fix linting issue with playwright report Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> * Rename typoed file Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> --------- Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> --------- Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com> Co-authored-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewer: @shakyShane
Asana: https://app.asana.com/0/1198964220583541/1204088031977626/f
Description
This prevents the focus event when clicking on Dax on extensions to avoid overlap with Chrome's native tooltips. It also fixes a couple of related bugs.
Steps to test
I'll add tests this week. Please advise if you can think of how to test the Dax click thingy. I was thinking of just specifying click coords in the Playwright tests, but if you have a simpler idea I'm all ears 👂🙂👂.